zend_compile: Bundle function type constants into an zend_function_type enum#21208
zend_compile: Bundle function type constants into an zend_function_type enum#21208TimWolla wants to merge 1 commit intophp:masterfrom
zend_function_type enum#21208Conversation
…ype` enum This clarifies the relationship between these constants and improves type safety a little.
TimWolla
force-pushed
the
zend-function-type
branch
from
a6c8db0 to
2fcddda
Compare
| #if __STDC_VERSION__ >= 202311L || defined(__cplusplus) | ||
| enum zend_function_type: uint8_t | ||
| #else | ||
| enum zend_function_type | ||
| #endif | ||
| { | ||
| ZEND_INTERNAL_FUNCTION = 1, | ||
| ZEND_USER_FUNCTION = 2, | ||
| ZEND_EVAL_CODE = 4, | ||
| }; | ||
|
|
||
| #if __STDC_VERSION__ >= 202311L || defined(__cplusplus) | ||
| typedef enum zend_function_type zend_function_type; | ||
| #else | ||
| typedef uint8_t zend_function_type; | ||
| #endif | ||
|
|
There was a problem hiding this comment.
It's better indeed, but I'd make it even more "compact". My motivation for this is the reduced "human parsing effort" overhead wrt preprocessor instructions.
| #if __STDC_VERSION__ >= 202311L || defined(__cplusplus) | |
| enum zend_function_type: uint8_t | |
| #else | |
| enum zend_function_type | |
| #endif | |
| { | |
| ZEND_INTERNAL_FUNCTION = 1, | |
| ZEND_USER_FUNCTION = 2, | |
| ZEND_EVAL_CODE = 4, | |
| }; | |
| #if __STDC_VERSION__ >= 202311L || defined(__cplusplus) | |
| typedef enum zend_function_type zend_function_type; | |
| #else | |
| typedef uint8_t zend_function_type; | |
| #endif | |
| #if __STDC_VERSION__ >= 202311L || defined(__cplusplus) | |
| typedef enum zend_function_type zend_function_type; | |
| enum zend_function_type: uint8_t | |
| #else | |
| typedef uint8_t zend_function_type; | |
| enum zend_function_type | |
| #endif | |
| { | |
| ZEND_INTERNAL_FUNCTION = 1, | |
| ZEND_USER_FUNCTION = 2, | |
| ZEND_EVAL_CODE = 4, | |
| }; |
There was a problem hiding this comment.
I initially tried that, but it didn't work for the C23 compiler. Don't have the error message at hand now, but it was something along the lines that the typedef implicitly defined the enum zend_function_type to be int-sized, which conflicted with the : uint8_t in the actual declaration. But AFAICT it is not possible to include the uint8_t within the (effective) forward-declaration in the typedef.
It could possibly become a bit cleaner if we would define a HAVE_C23_ENUM_SIZE or similar?
There was a problem hiding this comment.
https://godbolt.org/z/q9f77xe4z Just an idea, maybe something for zend_portability.h.
There was a problem hiding this comment.
May be the best if we end up doing more of those
3 participants
This clarifies the relationship between these constants and improves type safety a little.